Skip to content

[Kernel] Add ModelOpt FP4 Checkpoint Support #12520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Mar 12, 2025
Merged

[Kernel] Add ModelOpt FP4 Checkpoint Support #12520

merged 10 commits into from
Mar 12, 2025

Conversation

@pavanimajety
Copy link
Collaborator

@pavanimajety pavanimajety commented Jan 28, 2025 •
edited by github-actions bot
Loading

@github-actions
Copy link

github-actions bot commented Jan 28, 2025

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@mergify
Copy link

mergify bot commented Jan 31, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @pavanimajety.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Jan 31, 2025
@trevor-m trevor-m mentioned this pull request Feb 26, 2025
Merged
6 tasks
@pavanimajety pavanimajety changed the title [Kernel] Add ModelOpt NVFP4 Checkpoint Support [Kernel] Add ModelOpt FP4 Checkpoint Support Feb 27, 2025
@mergify mergify bot removed the needs-rebase label Feb 27, 2025
@mgoin mgoin self-assigned this Feb 27, 2025
@trevor-m trevor-m mentioned this pull request Feb 28, 2025
Merged
6 tasks
@pavanimajety pavanimajety force-pushed the modelopt-ckpt-nvfp4 branch 2 times, most recently from 1aaebfb to dd4fad2 Compare March 7, 2025 23:49
@pavanimajety pavanimajety marked this pull request as ready for review March 9, 2025 20:53
@pavanimajety
[Model] Add NVFP4 checkpoint support
c0c2cda 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
NFC
edf7348 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
Fix to use scaled_fp4_* names
117d7e8 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
FP4 quant
c0e3d6a 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
Further fixes for the fp4 checkpoint
a07e20b 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
ongoing changes
84d4653 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
Fix errors
0eb5cf1 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
Fix mypy errors
bc07ee7 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety pavanimajety force-pushed the modelopt-ckpt-nvfp4 branch from dd4fad2 to bc07ee7 Compare March 9, 2025 21:59
mgoin
mgoin reviewed Mar 9, 2025
View reviewed changes
Copy link
Member

@mgoin mgoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few comments thanks!

vllm/model_executor/layers/quantization/modelopt.py
Comment on lines 226 to 233
def get_quant_method(self, layer: torch.nn.Module,
prefix: str) -> Optional["QuantizeMethodBase"]:
from vllm.attention.layer import Attention # Avoid circular import
if isinstance(layer, LinearBase):
return ModelOptNvFp4LinearMethod(self)
elif isinstance(layer, Attention):
return ModelOptFp8KVCacheMethod(self)
return None
Copy link
Member

@mgoin mgoin Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we are missing the check between prefix and exclude_modules to ignore layers, as noted in the model config https://huggingface.co/nvidia/DeepSeek-R1-FP4/blob/761db7ea1b0b750e29fa78ebd2ce449e619809c5/hf_quant_config.json#L10-L15

Copy link
Collaborator Author

@pavanimajety pavanimajety Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeepseekR1-FP4 doesn't work with this PR(We are missing support for FP4 Group Gemm). I added back the exclude_modules and now load the linear layer classes accordingly. The actual correctness will be checked after adding support for the R1-FP4 model in a future PR.

vllm/model_executor/layers/quantization/modelopt.py
Comment on lines +381 to +384
# for input only the contracting dimension has a constraint.
x_m, _ = x.shape
w_n, _ = layer.weight.shape
output_shape = [x_m, w_n]
Copy link
Member

@mgoin mgoin Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the constraint? There seems to be none

Copy link
Collaborator Author

@pavanimajety pavanimajety Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this comment since we explicitly check the correctness of x's shape in scaled_fp4_quant. The constraint is that the innermost dimension must be divisible by block_size.

@pavanimajety
Address feedback and fix the last my-py error
7bb61d7 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
@pavanimajety
Copy link
Collaborator Author

pavanimajety commented Mar 9, 2025

Thank you for your review, @mgoin!

@pavanimajety pavanimajety requested a review from mgoin March 10, 2025 16:22
@WoosukKwon
Copy link
Collaborator

WoosukKwon commented Mar 11, 2025

@mgoin thanks for your review. can you please take another look?

mgoin
mgoin approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@mgoin mgoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @pavanimajety!

@mgoin mgoin added quantization ready ONLY add when PR is ready to merge/full CI is needed labels Mar 11, 2025
@mgoin mgoin enabled auto-merge (squash) March 11, 2025 22:15
@pavanimajety
Revert scale changes to qwen2
ab8c90a 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
auto-merge was automatically disabled March 12, 2025 02:02

Head branch was pushed to by a user without write access

@WoosukKwon WoosukKwon enabled auto-merge (squash) March 12, 2025 02:28
@WoosukKwon WoosukKwon merged commit debd6bb into vllm-project:main Mar 12, 2025
57 checks passed
richardsliu pushed a commit to richardsliu/vllm that referenced this pull request Mar 14, 2025
@pavanimajety @richardsliu
[Kernel] Add ModelOpt FP4 Checkpoint Support (vllm-project#12520)
8455e39 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: Richard Liu <ricliu@google.com>
lulmer pushed a commit to lulmer/vllm that referenced this pull request Apr 7, 2025
@pavanimajety @lulmer
[Kernel] Add ModelOpt FP4 Checkpoint Support (vllm-project#12520)
c433866 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: Louis Ulmer <ulmerlouis@gmail.com>
@ckhordiasma ckhordiasma mentioned this pull request Apr 17, 2025
Closed
shreyankg pushed a commit to shreyankg/vllm that referenced this pull request May 3, 2025
@pavanimajety @shreyankg
[Kernel] Add ModelOpt FP4 Checkpoint Support (vllm-project#12520)
5e4aed5 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
@pavanimajety
[Kernel] Add ModelOpt FP4 Checkpoint Support (vllm-project#12520)
4f630f1 
Signed-off-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgoin mgoin mgoin approved these changes

@DarkLight1337 DarkLight1337 Awaiting requested review from DarkLight1337 DarkLight1337 is a code owner

@ywang96 ywang96 Awaiting requested review from ywang96 ywang96 is a code owner

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

@tlrmchlsmth tlrmchlsmth Awaiting requested review from tlrmchlsmth tlrmchlsmth is a code owner

Assignees

@mgoin mgoin

Labels

quantization ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pavanimajety @WoosukKwon @mgoin